Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporary interface: Add fsm_determinise_with_config. #28

Closed
wants to merge 1 commit into from

Conversation

silentbicycle
Copy link

This interface will probably need some discussion / changes before it's ready to upstream. I think it's worth having some kind of limit here, since determinisation is most likely to have wide variation in resource usage.

  • The names (particularly the enum) are cumbersome here, but I would really rather have a distinct return value for exceeding the passed-in limit, rather than using something that nominally fits something from errno, like E2BIG.
  • It seems a bit silly to pass in a struct with just the one field, but as long as they have a reasonable default when zeroed, we could add other fields to the struct later without needing interface changes. We could just give fsm_determinise an extra step_limit argument, but if we're potentially going to add any other options at any point (logging flags?) then a struct would be less hassle.
  • I'm adding a separate function fsm_determinise_with_config to avoid an interface change with fsm_determinise directly (which is called all over the place). We could either change fsm_determinise (once) and take a struct, come up with a naming convention for <function>_with_an_extra_configuration_struct (fsm_determinise_with_config? fsm_determinise_cfg?) and keep both (and possibly add similar variants for other functions later), or just add a state_limit param, but that would be my last choice.

This interface will probably need some discussion / changes
before it's ready to upstream.
Copy link

@katef katef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be fine to go upstream

@silentbicycle
Copy link
Author

Closing, posted to upstream instead: katef#498

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants